-
Notifications
You must be signed in to change notification settings - Fork 521
WWSTCERT-8208 Add new Zigbee Power Meter Device #1999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
up |
| @@ -0,0 +1,373 @@ | |||
| -- Copyright 2022 SmartThings | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date
| device:set_field(constants.SIMPLE_METERING_MULTIPLIER_KEY, 1, {persist = true}) | ||
| device:set_field(constants.SIMPLE_METERING_DIVISOR_KEY, 100, {persist = true}) | ||
| device:set_field(constants.ELECTRICAL_MEASUREMENT_MULTIPLIER_KEY, 1, {persist = true}) -- ACPower | ||
| device:set_field(constants.ELECTRICAL_MEASUREMENT_DIVISOR_KEY, 1, {persist = true}) -- ACPower | ||
| device:set_field("RMSVOLTAGE_MULTIPLIER", 1, {persist = true}) | ||
| device:set_field("RMSVOLTAGE_DIVISOR", 100, {persist = true}) | ||
| device:set_field("RMSCURRENT_MULTIPLIER", 1, {persist = true}) | ||
| device:set_field("RMSCURRENT_DIVISOR", 100, {persist = true}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you set these fields to persist there's no reason to re-set them every time the driver starts
so either this should happen in added or configure, or they shouldn't be set to persist
| local function current_phaseA_handler(driver, device, value, zb_rx) | ||
| local multiplier = device:get_field("RMSCURRENT_MULTIPLIER") or 1 | ||
| local divisor = device:get_field("RMSCURRENT_DIVISOR") or 100 | ||
| local raw_value = value.value * multiplier/divisor -- Uint A | ||
| device:emit_component_event(device.profile.components["PhaseA"], capabilities.currentMeasurement.current({value = raw_value, unit = "A"})) | ||
| end | ||
|
|
||
| local function current_phaseB_handler(driver, device, value, zb_rx) | ||
| local component = device.profile.components["PhaseB"] | ||
| if component ~= nil then | ||
| local multiplier = device:get_field("RMSCURRENT_MULTIPLIER") or 1 | ||
| local divisor = device:get_field("RMSCURRENT_DIVISOR") or 100 | ||
| local raw_value = value.value * multiplier/divisor -- Uint A | ||
| device:emit_component_event(component, capabilities.currentMeasurement.current({value = raw_value, unit = "A"})) | ||
| end | ||
| end | ||
|
|
||
| local function current_phaseC_handler(driver, device, value, zb_rx) | ||
| local component = device.profile.components["PhaseC"] | ||
| if component ~= nil then | ||
| local multiplier = device:get_field("RMSCURRENT_MULTIPLIER") or 1 | ||
| local divisor = device:get_field("RMSCURRENT_DIVISOR") or 100 | ||
| local raw_value = value.value * multiplier/divisor -- Uint A | ||
| device:emit_component_event(component, capabilities.currentMeasurement.current({value = raw_value, unit = "A"})) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have a lot of duplicated code here. I would use a function factory like this: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/blob/main/drivers/SmartThings/zigbee-thermostat/src/init.lua#L355 to avoid this.
| device:send(SimpleMetering.attributes.CurrentSummationDelivered:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ActivePower:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.RMSVoltage:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.RMSCurrent:read(device)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the attribute has been added as a monitored attribute, it should automatically be read by refresh.
Please write some tests to verify this.
| supported_capabilities = { | ||
| capabilities.refresh, | ||
| capabilities.powerMeter, | ||
| capabilities.energyMeter, | ||
| capabilities.powerConsumptionReport, | ||
| capabilities.currentMeasurement, | ||
| capabilities.voltageMeasurement, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be removed. this field has no effect for sub-drivers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I made the modifications according to your requirements, please check the latest commit. Thanks!
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Channel deleted. |
Test Results 71 files 480 suites 0s ⏱️ Results for commit 2ec2d28. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 2ec2d28 |
|
|
Also, please add some unit tests. |
Hi, I've written some test uints. Could you please tell me how I can run these test uints locally before pull request to ensure they are working? |
|
@script0803 I'd first follow this guide: https://developer.smartthings.com/docs/devices/hub-connected/set-up-dev-env using the artifact containing the lua libraries here: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/releases/tag/apiv15_58 Then navigate to your driver's |
Hi, when testing the do_config part, the test uint receives some strange messages. I haven't used the frequency attribute in my driver, but it receives an unexpected report of frequency divisor. Additionally, there are similar attributes like ACPower Multiplier and Divisor, and their min interval, max interval, and report changeable are very strange. These messages are all identified as unexpected messages, which causes my device to fail the test. How can I solve this problem? test code test.register_coroutine_test(
"Device configure lifecycle event should configure device properly",
function()
test.socket.zigbee:__set_channel_ordering("relaxed")
test.socket.device_lifecycle:__queue_receive({ mock_device.id, "doConfigure" })
test.socket.zigbee:__expect_send({
mock_device.id,
zigbee_test_utils.build_bind_request(mock_device, zigbee_test_utils.mock_hub_eui, SimpleMetering.ID)
})
test.socket.zigbee:__expect_send({
mock_device.id,
zigbee_test_utils.build_bind_request(mock_device, zigbee_test_utils.mock_hub_eui, ElectricalMeasurement.ID)
})
test.socket.zigbee:__expect_send({
mock_device.id,
SimpleMetering.attributes.CurrentSummationDelivered:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ActivePower:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSVoltage:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSCurrent:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
cluster_base.configure_reporting(mock_device, data_types.ClusterId(SimpleMetering.ID), data_types.AttributeId(0x0001), data_types.ZigbeeDataType(SimpleMetering.attributes.CurrentSummationDelivered.base_type.ID), 30, 120, 0)
})
mock_device:expect_metadata_update({ provisioning_state = "PROVISIONED" })
end
) |
After trying, it was only by listing all of them that the do config test could be passed. However, there are many attributes in the list that have not been used by my devices. test.register_coroutine_test(
"Device configure lifecycle event should configure device properly",
function()
test.socket.zigbee:__set_channel_ordering("relaxed")
test.socket.device_lifecycle:__queue_receive({ mock_device.id, "doConfigure" })
test.socket.zigbee:__expect_send({
mock_device.id,
zigbee_test_utils.build_bind_request(mock_device, zigbee_test_utils.mock_hub_eui, SimpleMetering.ID)
})
test.socket.zigbee:__expect_send({
mock_device.id,
zigbee_test_utils.build_bind_request(mock_device, zigbee_test_utils.mock_hub_eui, ElectricalMeasurement.ID)
})
test.socket.zigbee:__expect_send({
mock_device.id,
SimpleMetering.attributes.CurrentSummationDelivered:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ActivePower:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSVoltage:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSCurrent:configure_reporting(mock_device, 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
cluster_base.configure_reporting(mock_device, data_types.ClusterId(SimpleMetering.ID), data_types.AttributeId(0x0001), data_types.ZigbeeDataType(SimpleMetering.attributes.CurrentSummationDelivered.base_type.ID), 30, 120, 0)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ACPowerDivisor:configure_reporting(mock_device, 1, 43200, 1)
})
test.socket.zigbee:__expect_send({
mock_device.id,
SimpleMetering.attributes.InstantaneousDemand:configure_reporting(mock_device, 5, 3600, 5)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ACPowerMultiplier:configure_reporting(mock_device, 1, 43200, 1)
})
test.socket.zigbee:__expect_send({
mock_device.id,
SimpleMetering.attributes.InstantaneousDemand:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
cluster_base.read_attribute(mock_device, data_types.ClusterId(SimpleMetering.ID), data_types.AttributeId(0x0001))
})
test.socket.zigbee:__expect_send({
mock_device.id,
SimpleMetering.attributes.CurrentSummationDelivered:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ActivePower:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSVoltage:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.RMSCurrent:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ACPowerMultiplier:read(mock_device)
})
test.socket.zigbee:__expect_send({
mock_device.id,
ElectricalMeasurement.attributes.ACPowerDivisor:read(mock_device)
})
mock_device:expect_metadata_update({ provisioning_state = "PROVISIONED" })
end |
|
@script0803 can you include your test in the PR here so I can take a look? |
Of course, thank you very much for your help! |
| [1] = { | ||
| id = 1, | ||
| manufacturer = "BITUO TECHNIK", | ||
| model = "SDM02", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you are missing an X at the end of this model name.
|
Your configuration test failures are because your fork needs to be rebased. There have been upstream changes in the main repo that are not included in your fork. |
|
@script0803 Please rebase your pull request to remove the merge conflicts. |
Check all that apply
Type of Change
Checklist
Description of Change
Add new Zigbee Power Meter Device.
Based on the existing driver, support for single-phase, two-phase, and three-phase energy meters has been added.
Added Capability: voltageMeasurement
Cluster: 0x0B04
Attributes: 0x0505, 0x0905, 0x0A05
Added Capability: currentMeasurement
Cluster: 0x0B04
Attributes: 0x0508, 0x0908, 0x0A08
Added Capability: powerMeter (extended for two-phase and three-phase applications)
Cluster: 0x0B04
Attributes: 0x050B, 0x090B, 0x0A0B
Added Capability: energyMeter (using component marked as producedEnergy)
Cluster: 0x0702
Attribute: 0x0001
Additionally, all types of Power Meters now support the capability: powerConsumptionReport, in preparation for integration with SmartThings Energy.
These changes have been communicated in advance to partners@smartthings.com. After the merge request, the WWST Certification process will be initiated.
Summary of Completed Tests